Improve the Type consistency of Special Functions (Fixes #17474) #18584

oxinabox · 2016-09-19T16:02:57Z

As I found in #17474,
the various special functions tend to just blindly cast everything to Float64.
Regardless of if it makes sense.
This is my attempt to fix that.
And while I am at it, to make the return types also a little more consistent across the functions.

~~This is WIP, I'm not actually sure my tests pass, but I would like feedback as to if I am going about this in a sane way.~~

Though these do need to move to SpecialFunctions.jl
(cf: JuliaMath/SpecialFunctions.jl#12)

tkelman

note the whitespace error

tkelman · 2016-09-20T01:12:56Z

base/special/gamma.jl

+    x = float(z)
+    t = Int(s)
+    if typeof(x) === typeof(z) && typeof(t) === typeof(s)
+        throw MethodError(zeta,(s,t))


throw(...)

and why is this a MethodError exactly? could use a comment

Thanks.

It is a method error because this is a fallback definition.
If the most specific implementation of this is foo(::Number),
and if once you convert it to a float, the most specific implementation is still foo(::Number),
because the type hasn't changed,
then we don't have a working implementation for this type.

This is the case for trigamma(::BigFloat)

I'll try and put that into a comment.
Should I write a block comment before all the fallback methods?
or write one per throw?

tkelman · 2016-09-20T01:15:02Z

base/special/gamma.jl

 function invdigamma(y::Float64)
-    # Closed form initial estimates
+   # Implementation of fixed point algorithm described in
+   # "Estimating a Dirichlet distribution" by Thomas P. Minka, 2000


don't change the indent of comments

I moved the comment into the inside of the function it was commenting; So it would not obscure the doc-string.
Should I not indent comments, that are inside functions? (honest question, I'm not sure on the convention)

I kinda think maybe I should have just added that comment to the doc-string.

if it's inside a function then it should be indented to the same level that it would be if it were code on the same line, generally - so a multiple of 4 spaces here

tkelman · 2016-09-20T01:16:18Z

test/math.jl

+@test typeof(digamma(big"2")) == BigFloat
+@test_throws MethodError trigamma(big"2")
+@test_throws MethodError trigamma(big"2.0")
+@test_throws MethodError invdigamma(big"2")


are these not available in mpfr/gmp?

Correct. They are not available, or if they are, then we are not importing them.
Let me check mpfr docs

Edit:
They are not available in MPFR:
http://www.mpfr.org/mpfr-current/mpfr.html#index-Special-functions
GMP doesn't have it since (as I understand it) these functions almost never return integers.

oxinabox · 2016-09-20T05:33:44Z

I need to make test cases for
eta{Complex{BigInt}, eta{Complex{BigFloat}, zeta{Complex{BigInt}, zeta{Complex{BigFloat},

to make sure they throw method errors, or work correctly.
And don't just dispatch to eta{Complex{Float64}} plus casting to BigFloat

And for my future reference: the test cases can use:

julia> eta(Complex(big"52.0")) |> real |> BigFloat  #A 
9.999999999999997779553950749686919152736663818359375000000000000000000000000000e-01

julia> eta(big"52.0") #B
9.999999999999997779553952297414836228661208246910286674974797896945379409710455e-01

julia> eta(Complex(52.0)) |> real |> BigFloat #C
9.999999999999997779553950749686919152736663818359375000000000000000000000000000e-01

It should be A==B not C, or a Method Error

Similarly:

julia> zeta(Complex(big"52.0")) |> real  #A 
1.000000000000000222044604925031308084726333618164062500000000000000000000000000

julia> zeta((big"52.0"))  #B 
1.000000000000000222044605079804198399932009420465396423665432943893439236654051

julia> zeta(Complex(52.0)) |> real |> BigFloat #C 
1.000000000000000222044604925031308084726333618164062500000000000000000000000000

Again A==B not C, or a Method Error

oxinabox · 2016-09-21T04:52:19Z

Thinking exactly how I want everything to behave.

The functions in gamma.jl above are only defined for Float64,
but through conversion we expect them to work for every Number
4 Cases:

A. Input is Float64 - use the above methods
B. Input is reasonably converted to Float64
- Convert input to Float64 and use case A, i
- then convert result input type
C. Input is not neither of the above (eg BigFloat),
- but does provice its own implementation (eg in mpfr.jl)
- then we use of course use that method
D. Input is not reasonably converted to a Float64,
- then use float to convert it
- D.1 If this produces a type that is not the same as the input type:
  - Then try and use case C, which may not exist (an so may method error)
- D.2 If this does not change type (i.e. it was already a abstract float
  - then this is a method error.

CarloLucibello · 2016-09-21T09:48:32Z

base/special/gamma.jl

-#  "Estimating a Dirichlet distribution" by Thomas P. Minka, 2000
+for f in (:zeta, :eta)
+    @eval begin
+        $f{T<:Union{Base.BitInteger,Float32,Float16}}(x::t) = oftype(float(x), $f(Complex128(z)))


Complex128(x) ?

Complex128 is a type alias for Complex{Float64}.
I guess cos it is made of 2 Float64s
It is odd that it is converting a real to a complex though.

Looking at it, it is because that line doesn't even compile. (I got distracted when writing this function i guess).
I know I've fixed it already in my local copy.
x::t is x::Complex{T}
Or probably x::Complex{Union{Base.BitInteger,Float32,Float16}}
Can't remember

sorry, I was very cryptic, I meant z should be changed to x in Complex128(z) :) but of course you already fixed that

oxinabox · 2016-10-05T13:04:28Z

Sorting this was a bit more complex that I thought.
I think I have it right now.
I am a little dubious about the methods I used to do so.
Like the ComplexOrRealUnion function.

I have moved it from WIP into RFC.
It does pass all the tests from issue #17474
but I'm not sure anymore that those tests were enough, as they word focusing only on single argument functions.
And the multiple argument functions are the ones that were particularly hard to correct.
I've added more tests, but maybe I should have more

oxinabox · 2016-10-06T02:16:24Z

base/special/gamma.jl

+ofpromotedtype(as::Tuple, c) = convert(promote_type(typeof.(as)...), c)
+ComplexOrRealUnion(TS...) = Union{(ComplexOrReal{T} for T in TS)...}
+
+const types_le_Float64 = (Float16, Float32, Float64, Base.BitInteger.types...)


Is this a good name for this?
It is the collection of types that are less-than or equal to precise than Float64, at representing Float64 inputs.
I don't think this follows naming convention

I'd probably just use the tuple in the loops and avoid defining the variable. It's only used three times.

oxinabox · 2016-10-06T02:17:26Z

base/special/gamma.jl

+# they also implement there own versions of the functions
+
+
+ofpromotedtype(as::Tuple, c) = convert(promote_type(typeof.(as)...), c)


Should this move to promotion.jl ?
It is the merging of oftype with promote

If you use the strategy I outlined (which might not work) then I don't think this function is necessary. I doubt that it will find much use outside this function anyway, so I think it can stay here if it ends up being useful.

oxinabox · 2016-10-06T02:18:54Z

base/special/gamma.jl

+
+
+ofpromotedtype(as::Tuple, c) = convert(promote_type(typeof.(as)...), c)
+ComplexOrRealUnion(TS...) = Union{(ComplexOrReal{T} for T in TS)...}


I would rather this was ComplexOrRealUnion{TS...} or something like that.
But I can't seem to get typealias to take a variable number of arguements

Maybe split the definitions into two line to avoid this function. It might be easier to read.

oxinabox · 2016-10-06T02:21:59Z

base/special/gamma.jl

+    if typeof(t) === typeof(s) && typeof(x) === typeof(z)
+        # There is nothing to fallback to, since this didn't work
+        throw(MethodError(zeta,(s,z)))
+    end


I think this should have a second fallback to zeta(::Number, Number) maybe.
Or possibly to prevent looping just try and convert s to a float and see how that goes.

See my other comment about the zeta.

oxinabox · 2016-10-08T05:05:31Z

Is there anything I need to do now, for this to proceed?
I posted a bit of a self-review highlighting what I thought might be issues.
I need further feedback and suggestion on how things should be changed

tkelman · 2016-10-08T11:39:18Z

base/special/gamma.jl

@@ -106,7 +108,8 @@ function lgamma(z::Complex{Float64})
                               -2.2315475845357937976132853e-04,9.9457512781808533714662972e-05,
                               -4.4926236738133141700224489e-05,2.0507212775670691553131246e-05)
    end
-    # use recurrence relation lgamma(z) = lgamma(z+1) - log(z) to shift to x > 7 for asymptotic series
+    # use recurrence relation lgamma(z) = lgamma(z+1) - log(z )


unnecessary space in log(z )

fixed, thanks

oxinabox · 2016-10-30T00:44:59Z

@tkelman Is there anything I need to do, that is holding this back?
I basically have not touched it for 3 weeks (other than yesterday's rebase)

tkelman · 2016-10-30T10:24:05Z

someone who knows this code should respond to your inline questions

oxinabox · 2016-10-31T00:34:44Z

someone who knows this code should respond to your inline questions

@andreasnoack you wrote a lot of this 2 years back; right?
At least the functions being called in the end, if not all the dispatches too them.

tkelman · 2016-10-31T00:37:59Z

base/special/gamma.jl

+# and results converted back. Similar for BitIntegers (eg Int32), but no converting back
+# Otherwise, we need to make things use their own `float` converting methods
+# and in those cases, we do not convert back either as we assume
+# they also implement there own versions of the functions


tkelman · 2016-11-01T11:15:37Z

base/special/gamma.jl

+# Otherwise, we need to make things use their own `float` converting methods
+# and in those cases, we do not convert back either as we assume
+# they also implement their own versions of the functions, with the correct return types.
+# Otherwise, if they do not implement their version of the functions we 


whitespace error

whitespace was fixed, implementation still needs review though

oxinabox · 2016-11-06T02:22:05Z

@simonbyrne @stevengj this basically implements what we discussed in #17474

tkelman · 2016-11-06T02:50:28Z

test/math.jl

+
+@test Base.Math.f64(complex(1f0,1f0)) == complex(1.0, 1.0)
+@test Base.Math.f64(1f0) == 1.0
+@test Base.Math.ofpromotedtype((complex(1f0, 1f0), 1.0), 5.0) == complex(5.0)


should these check types too with === ?

Yes, they should. (I will change)

andreasnoack · 2016-10-31T01:31:23Z

base/special/gamma.jl

+    oftype(z, polygamma(m, f64(z)))
+end
+
+for T1 in types_le_Float64, T2 in types_le_Float64


I think meta programming sometimes can be a bit convoluted to read. Would it be simpler to define something like

for T in (Float16,Float32,Complex{Float16},Complex{Float32}) zeta(x::T, y::T) = T(zeta(f64(x),f64(y)))` end zeta(x::Integer,y::Integer) = zeta(f64(x),f64(y)) zeta(x::Number, y::Number) = zeta(promote(x,y)...) zeta(x::BigFloat,y::BigFloat) = error

This is indeed a particularly convoluted piece of metaprogramming.
I rewrote it about 3 times in different ways.
but I think I'll try again.
Get rid of some of the if T1<:Integer && T2<:Integer
by using a separate loop for ints.

I have been trying to avoid things like
zeta(x::BigFloat,y::BigFloat) = error..., because that will mean that if someone does write a zeta(::BigFloat, ::BigFloat)
it will have issues with overwriting a function name. And will trigger #265 problems.

In general, rather than exclude the types in Base that a function does not work for,
I am trying to include only the types in Base which a function does work for.
Which should make extending with custom number types from packages sane.

Since float(::BitInt)::BigFloat, wouldn't the version below accomplish what you want?

for T in (Float16,Float32,Complex{Float16},Complex{Float32}) zeta(x::T, y::T) = T(zeta(f64(x),f64(y)))` end zeta(x::Integer,y::Integer) = zeta(float(x),float(y)) zeta(x::Number, y::Number) = zeta(promote(x,y)...) zeta(x::BigFloat,y::BigFloat) = error

Didn't see your response before I made mine here

I think the problem is that a line like zeta(x::Number, y::Number) = zeta(promote(x,y)...) is really simple and easy to read relative to the implementation suggested in the PR. The problem with the version I suggest is, of course, that it is an infinite recursion if no promotion happens. Therefore the explicit error for BigFloat. Rumors are that #265 is close to getting fixed so maybe the redefinition concern shouldn't weight too much.

Did you see my implementation of: function zeta(s::Number, z::Number) etc?
Line 694.
This doesn't suffer from infinite recursion.
I might be able to delete quiet a few definitions, and just fall back to it.
(It might need a promote added after the float)

It might end up a lot like yours, I think.
But avoid the explict erroring for BigFloats.

That could work. If you restrict the method in 694 to promote to the same element type for the two arguments. Then the method here can be restricted to a single loop instead of looping through the combinations of both argument types.

Can zeta's first argument just always be a Float64?
I think it can.
I don't think there is a meaningful difference to giving it Int (maybe a microspeed up).

If it can't then promoting to the same type won't work.
But I think it can,
so that is find, and Integers can be treated as just numbers, when in the first arg position (as well as in the second).

Yes. I think it is fine to remove Int from the signature.

andreasnoack · 2016-10-31T01:33:56Z

base/special/gamma.jl

+# they also implement there own versions of the functions
+
+
+ofpromotedtype(as::Tuple, c) = convert(promote_type(typeof.(as)...), c)


If you use the strategy I outlined (which might not work) then I don't think this function is necessary. I doubt that it will find much use outside this function anyway, so I think it can stay here if it ends up being useful.

andreasnoack · 2016-10-31T01:36:25Z

base/special/gamma.jl

+
+
+ofpromotedtype(as::Tuple, c) = convert(promote_type(typeof.(as)...), c)
+ComplexOrRealUnion(TS...) = Union{(ComplexOrReal{T} for T in TS)...}


Maybe split the definitions into two line to avoid this function. It might be easier to read.

andreasnoack · 2016-10-31T01:37:27Z

base/special/gamma.jl

+ofpromotedtype(as::Tuple, c) = convert(promote_type(typeof.(as)...), c)
+ComplexOrRealUnion(TS...) = Union{(ComplexOrReal{T} for T in TS)...}
+
+const types_le_Float64 = (Float16, Float32, Float64, Base.BitInteger.types...)


I'd probably just use the tuple in the loops and avoid defining the variable. It's only used three times.

andreasnoack · 2016-10-31T01:38:32Z

base/special/gamma.jl

+
+for f in (:digamma, :trigamma, :zeta, :eta, :invdigamma)
+    @eval begin
+        $f(z::ComplexOrRealUnion(Base.BitInteger.types...)) = $f(f64(z))


Are we sure that it is so dangerous to convert all integers to Float64?

The problem is not accurasy (for real Ints at least), it is inconsistant return types.

Loss of accuracy does not occur until beyond ints 2^53 -- which I'm yet to find a case that it matters for any of these functions, as they tend to hit an asymptote long before then

We want BitInteger (Int64 etc) to have Float64 results from these functions (at least that is current behavior).

The main deal is we want BigInts to have BigFloat results if possible,
and to throw method errors if not (rather than having Float64 results).
Which is most of #17474

Eg right now

julia> zeta(big"52") 1.0000000000000002 julia> zeta(big"52.0") 1.000000000000000222044605079804198399932009420465396423665432943893439236654051

We really want to avoid reporting BigFloat results, that were calculated with Float64's behind the scenes, and then converted. as in the examples in #18584 (comment)
I don't think that kinda thing ever happens for BigInt, right now.
Because right now most BigIntegers functions return Float64s
Even if there is more accurate BigFloat method available in MPFR

Got it. Thanks for explaining. See my updated "proposal" below.

andreasnoack · 2016-10-31T01:38:56Z

base/special/gamma.jl

+    if typeof(t) === typeof(s) && typeof(x) === typeof(z)
+        # There is nothing to fallback to, since this didn't work
+        throw(MethodError(zeta,(s,z)))
+    end


See my other comment about the zeta.

andreasnoack · 2016-11-06T10:33:37Z

@oxinabox Sorry for submitting my comments so late. I made them a while ago and thought they were visible. Still adapting to the review system.

oxinabox · 2016-11-21T03:36:17Z

Right, sorry for the delay.

I think I have made most of @andreasnoack 's suggestions that we ended up with.

ofpromotedtype is gone
ComplexOrRealUnion is gone -- but ComplexOrReal remains (should I get rid of that too?)
Complicated meta programming gone, by removing the redundant special asing of BitIntegers
As discusses the base definition of zeta(s,z) now requires s to be a Float64, which as no visible effects, because underflow will occur long before the point where there is no exact Float64 representation of an integer.

Also now the return type of zeta(s,z) depends only on the type of z, and whether or not s is Complex -- not on the precision of s

andreasnoack · 2016-11-21T10:55:32Z

base/special/gamma.jl

+# It is safe to convert `s` to a float as underflow will occur before precision issues
+zeta(s::Integer, z::Number) = zeta(oftype(z, s), z)
+function zeta(s::Integer, z::Integer)
+	x=float(z)


tabs -> spaces

ah, bother. I redid my vimrc, and must have forgotten to change that back.
Thanks

andreasnoack · 2016-11-21T11:09:50Z

base/special/gamma.jl

+		ζ = zeta(f64(s), f64(z))
+
+		if typeof(s) <: Complex || typeof(z) <: Complex
+			convert(Complex{$T2}, ζ)


Shouldn't it be promote_type($T1,$T2} instead of of $T2?

It used to be, but after thinking a bit more about zeta,
I started to think that it should maybe be driven only by the type $T2.
But I now think I was going down the wrong line.

I wanted zeta(::BigInteger, ::Float64) :: Float64 (which is what depending only on $T2 gives me)

promote_type($T1,$T2}, gives me zeta(::BigInteger, ::Float64) :: BigFloat -- which is a lie as that precision is not there.

I now think that it should just MethodError, which I can get by deleting a definition (or two)

+1 to MethodError if either argument is Big.

oxinabox · 2016-11-21T15:41:36Z

if you were okay with the StackOverflowError.

StackOverflowErrors are really annoying to debug.
They thrash the Call Stack, making it hard to workout what caused them.
I'ld rather not have any method in Base being able to be put into such a loop just by getting a parameter wrong -- and wrong in a way that isn't clear, since we have a method zeta(::Number, ::Number) that shows up when you enter methods(zeta)

andreasnoack · 2016-11-22T14:58:12Z

I'ld rather not have any method in Base being able to be put into such a loop just by getting a parameter wrong -- and wrong in a way that isn't clear, since we have a method zeta(::Number, ::Number) that shows up when you enter methods(zeta)

I think this applies to the current version as well. There you'll get a MethodError even though a method is defined according to methods(zeta).

To summarize, I think there are three solutions

Use the promotion mechanisms in Julia to get concise code but with StackOverflowErrors
Same as 1. + explicit errors for BigFloat methods
Avoid the promotion mechanisms in Julia and define methods one by one possibly with meta programming (this PR)

I recognize the problem with StackOverflowErrors but if it wasn't for them, I'd prefer 1. I'm wondering if it would be technically possible to add an annotation to a Julia function to make non-recursive. Then promote wouldn't have to cause a stack overflow.

simonbyrne · 2016-11-22T15:53:39Z

Note that we do manually throw(MethodError()) elsewhere, e.g.:

julia/base/special/bessel.jl

Line 452 in 68af05e

$bfn{T<:AbstractFloat}(k::T, z::Complex{T}) = throw(MethodError($bfn,(k,z)))

It's not pretty, but I think it is the least worst option.

oxinabox · 2016-11-24T02:08:14Z

Note that we do manually throw(MethodError()) elsewhere, ...
It's not pretty, but I think it is the least worst option.

I wonder if MethodError should have some parameter to indicate that it was manually thrown, rather than by the dispatch system.
Then we can avoid showing the list of methods, and can give a different error message explaining it.

vtjnash · 2016-11-24T17:16:12Z

#17057 will include that flag :)

tkelman · 2016-11-25T12:39:06Z

base/special/gamma.jl

-Compute the polygamma function of order `m` of argument `x` (the `(m+1)th` derivative of the
-logarithm of `gamma(x)`)
+Compute the polygamma function of order `m` of argument `x`
+(the `(m+1)th` derivative of the logarithm of `gamma(x)`)


"th" shouldn't be code highlighted

simonbyrne · 2016-11-25T14:10:51Z

the other trick is to have a different function, e.g. _zeta which gets called:

zeta(x::Number, y::Number) = _zeta(float(x), float(y))

musm · 2017-01-16T05:06:35Z

bump

oxinabox · 2017-02-04T10:00:44Z

I think this is still good to go.
everything from #18584
was resolved months ago when @andreasnoack comment was made.
(Given the concensus that a stack overflow error was not a good idea.)

Unless the suggestion from @simonbyrne in #18584
is desired.
Which is not a bad idea; but I don't want to implement this.
If that is the way forward, it will have to be by someone else.
Who can make a new PR and look onto this one for guidance.

Rebasing is getting harder the more time passes.

tkelman · 2017-02-04T10:16:20Z

Thanks for the reminders. Especially relevant as there are few open PR's that move this code around but aren't intending to change behavior. cc @ararslan, what do we want to do here short-term?

ararslan · 2017-02-04T18:43:56Z

@tkelman Tough call, as obviously a lot of work has gone into this PR and I've also put a lot of work into moving these things out. I guess it depends on whether getting this stuff out for 0.6 is a priority. If so, I'd say this should be reopened against SpecialFunctions (a nontrivial task), otherwise I'd say merge this and I can update things accordingly.

tkelman · 2017-02-09T19:51:02Z

Since the package is now registered and #20427 is just about ready, I'm going to assign @ararslan as a next-step to port this over to the package version. Thanks for the contribution and sorry this didn't get merged a few months ago, we'll get these improvements into the package (where people even on release Julia can use it right away).

kshyatt added maths Mathematical functions types and dispatch Types, subtyping and method dispatch labels Sep 19, 2016

tkelman previously requested changes Sep 20, 2016

View reviewed changes

CarloLucibello reviewed Sep 21, 2016

View reviewed changes

oxinabox force-pushed the master branch from e9dd10e to 552a0f9 Compare October 5, 2016 12:57

oxinabox changed the title ~~[WIP] Improve the Type consistency of Special Functions (Fixes #17474)~~ [RFC] Improve the Type consistency of Special Functions (Fixes #17474) Oct 5, 2016

oxinabox changed the title ~~[RFC] Improve the Type consistency of Special Functions (Fixes #17474)~~ RFC: Improve the Type consistency of Special Functions (Fixes #17474) Oct 6, 2016

oxinabox commented Oct 6, 2016

View reviewed changes

tkelman reviewed Oct 8, 2016

View reviewed changes

oxinabox force-pushed the master branch from 904d5cb to 9c51b1e Compare October 29, 2016 08:16

oxinabox changed the title ~~RFC: Improve the Type consistency of Special Functions (Fixes #17474)~~ Improve the Type consistency of Special Functions (Fixes #17474) Oct 29, 2016

tkelman reviewed Oct 31, 2016

View reviewed changes

tkelman reviewed Nov 1, 2016

View reviewed changes

tkelman reviewed Nov 6, 2016

View reviewed changes

andreasnoack reviewed Nov 6, 2016

View reviewed changes

oxinabox force-pushed the master branch from 96fad0f to 2b3557a Compare November 21, 2016 03:28

andreasnoack reviewed Nov 21, 2016

View reviewed changes

oxinabox added 12 commits November 24, 2016 10:28

=Work on JuliaLang#17474

b0b58a8

=Work on JuliaLang#17474 squashme

ba62160

=Tests Passing

bf783c9

=revert changes to sysimg (rebase these out after)

63fc253

=Fix whitespace and line wrap

f62b049

=haul in master (something went wrong in rebase?)

dca8256

=correct MethodErrors for JuliaLang#17474

c96f578

=fix whitespace

706773b

=More comments explaining convert strat

5a11d1d

=whitespace [ci skip]

f057335

=remove unrequired special casing for bitintegers

6334dda

=remove last of the special casing for Integers

e81314f

oxinabox force-pushed the master branch from 2b3557a to e81314f Compare November 24, 2016 02:28

tkelman reviewed Nov 25, 2016

View reviewed changes

=fix formatting in docstring

0f62f12

Merge branch 'master' into master

9fd1a32

=remove whitespace

734ca54

tkelman mentioned this pull request Feb 9, 2017

Port "Improve the Type consistency of Special Functions" pull request here JuliaMath/SpecialFunctions.jl#17

Closed

tkelman closed this Feb 9, 2017

		# they also implement there own versions of the functions


		ofpromotedtype(as::Tuple, c) = convert(promote_type(typeof.(as)...), c)



		ofpromotedtype(as::Tuple, c) = convert(promote_type(typeof.(as)...), c)
		ComplexOrRealUnion(TS...) = Union{(ComplexOrReal{T} for T in TS)...}

Improve the Type consistency of Special Functions (Fixes #17474) #18584

Improve the Type consistency of Special Functions (Fixes #17474) #18584

Conversation

oxinabox commented Sep 19, 2016 • edited Loading

tkelman left a comment

Choose a reason for hiding this comment

tkelman Sep 20, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oxinabox Sep 20, 2016 • edited Loading

Choose a reason for hiding this comment

oxinabox commented Sep 20, 2016

oxinabox commented Sep 21, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oxinabox commented Oct 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oxinabox commented Oct 8, 2016

Choose a reason for hiding this comment

oxinabox Oct 9, 2016 • edited Loading

Choose a reason for hiding this comment

oxinabox commented Oct 30, 2016

tkelman commented Oct 30, 2016

oxinabox commented Oct 31, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oxinabox commented Nov 6, 2016

Choose a reason for hiding this comment

oxinabox Nov 6, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreasnoack commented Nov 6, 2016

oxinabox commented Nov 21, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oxinabox commented Nov 21, 2016

andreasnoack commented Nov 22, 2016 • edited Loading

simonbyrne commented Nov 22, 2016

oxinabox commented Nov 24, 2016

vtjnash commented Nov 24, 2016

Choose a reason for hiding this comment

simonbyrne commented Nov 25, 2016

musm commented Jan 16, 2017

oxinabox commented Feb 4, 2017 • edited Loading

tkelman commented Feb 4, 2017

ararslan commented Feb 4, 2017

tkelman commented Feb 9, 2017

oxinabox commented Sep 19, 2016 •

edited

Loading

tkelman Sep 20, 2016 •

edited

Loading

oxinabox Sep 20, 2016 •

edited

Loading

oxinabox Oct 9, 2016 •

edited

Loading

oxinabox Nov 6, 2016 •

edited

Loading

andreasnoack commented Nov 22, 2016 •

edited

Loading

oxinabox commented Feb 4, 2017 •

edited

Loading